Skip to content

Conversation

@ppetermsc
Copy link

No description provided.

.idea/.gitignore Outdated

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⛔️Нужно исправить. Папку .idea не нужно было загружать в репозиторий. Эта папка должна быть добавлена в .gitignore.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Исправлено.


@RunWith(Parameterized.class)
public class BunTest {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⛔️Нужно исправить. Тестируем только класс Burger

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Исправлено, класс удален.

String receipt = burger.getReceipt();

assertNotNull("Чек не должен быть null", receipt);
assertTrue("Чек должен содержать название булочки", receipt.contains(bunName));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⛔️Нужно исправить. Для юнит-тестов придерживаемся подхода: один тест, значит одна проверка. Если очень хочется несколько проверок -- тогда используем softAssertions. Поправь, пожалуйста, во всем коде

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Исправлено в двух тестовых классах.


assertNotNull("Чек не должен быть null", receipt);
assertTrue("Чек должен содержать название булочки", receipt.contains(bunName));
assertTrue("Чек должен содержать название ингредиента", receipt.contains(ingredientName.toLowerCase()));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⛔️Нужно исправить. Строку рецепта проверяем целиком, чтобы не пропустить ошибки форматирования

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Исправлено.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

не исправлено

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Исправлено. Проблема в том, что решая конфликты с Гитом, по ошибке откатил коммиты на один из предыдущих и отправил на проверку старую версию! В новой этот вопрос решен.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⛔️Нужно исправить. В папке target оставляем только отчет, остальные файлы нужно удалить из пулл реквеста

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Исправлено.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Файлы отчета оставлены в папке таргет, остальные удалены.

- Use updated BurgerReceiptTest with full receipt validation
- Use updated BurgerTest with single assertion principle
- Remove BunTest as not required by specification
- Add .gitignore for proper file exclusion
- Keep local README.md version
- Remove target/ folder (build artifacts)
- Remove .idea/ folder (IDE settings)
- Keep only source code and configuration files

String receipt = burger.getReceipt();

assertNotNull("Чек не должен быть null", receipt);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⛔️Нужно исправить. Для юнит-тестов придерживаемся подхода: один тест, значит одна проверка. Если очень хочется несколько проверок -- тогда используем softAssertions. Поправь, пожалуйста, во всем коде

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Исправлено. Отправил старую версию по ошибке.


assertNotNull("Чек не должен быть null", receipt);
assertTrue("Чек должен содержать название булочки", receipt.contains(bunName));
assertTrue("Чек должен содержать название ингредиента", receipt.contains(ingredientName.toLowerCase()));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

не исправлено

burger.addIngredient(ingredient2);
burger.removeIngredient(0);

assertEquals("Должен остаться один ингредиент после удаления", 1, burger.ingredients.size());

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⛔️Нужно исправить. Для юнит-тестов придерживаемся подхода: один тест, значит одна проверка. Если очень хочется несколько проверок -- тогда используем softAssertions. Поправь, пожалуйста, во всем коде

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Исправлено в актуальной версии.

@Test
public void testMoveIngredient() {
Burger burger = new Burger();
Ingredient ingredient1 = new Ingredient(ingredientType, ingredientName, ingredientPrice);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⛔️Нужно исправить. При нейминге не рекомендуется использовать числа (Field2), их еще называют magicNumbers. Очень тяжело поддерживать код с magicNumbers.

public void testMoveIngredient() {
Burger burger = new Burger();
Ingredient ingredient1 = new Ingredient(ingredientType, ingredientName, ingredientPrice);
Ingredient ingredient2 = new Ingredient(IngredientType.FILLING, "сосиска", 300f);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⛔️Нужно исправить. Нужно использовать моки

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Сделано.

assertNotNull("Чек не должен быть null", receipt);
assertTrue("Чек должен содержать название булочки", receipt.contains(bunName));
assertTrue("Чек должен содержать название ингредиента", receipt.contains(ingredientName.toLowerCase()));
assertTrue("Чек должен содержать цену", receipt.contains("Price:"));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⛔️Нужно исправить. Тесты дважды в разных пакетах лежат

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Убрал лишние папки из коммита. Мусор оказался там, когда решал конфликты с гитом и откатил не на ту версию.

@Test
public void testAddIngredient() {
Burger burger = new Burger();
Ingredient ingredient = new Ingredient(ingredientType, ingredientName, ingredientPrice);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⛔️Нужно исправить. Нужно моки использовать

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Сделано.

@Test
public void testRemoveIngredient() {
Burger burger = new Burger();
Ingredient ingredient1 = new Ingredient(ingredientType, ingredientName, ingredientPrice);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⛔️Нужно исправить. Для юнит-тестов придерживаемся подхода: один тест, значит одна проверка. Если очень хочется несколько проверок -- тогда используем softAssertions. Поправь, пожалуйста, во всем коде

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Сделано.


@Test
public void testMoveIngredient() {
Burger burger = new Burger();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⛔️Нужно исправить. Нужна ли в этих тестах параметризация ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Оставил только там, где она необходима.

assertNotNull("Булочка должна быть выбрана", burger.bun);
assertEquals("Булочка должна совпадать", bun, burger.bun);
}
} No newline at end of file

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⛔️Нужно исправить. Нет отчёта о тестировании

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Отчет в папке таргет, остальные файлы из коммита убрал.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⛔️Нужно исправить. Нужно приложить все файлы из папки jacoco

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ОК. Сделано.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants